Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add PacketSerde interface and expand PacketUtils for more wpimath classes #1058

Merged
merged 8 commits into from
Dec 25, 2023

Conversation

srimanachanta
Copy link
Member

Follows a similar system to the current Protobuf implementation that helps make code more readable and expandable. Also wraps the NT topic to be more useful. Impl stuff is hidden so it can't be extended. Optimizes AT-specific classes by only serializing data when needed, won't save on size but will on time.

closes #1003

@mcm001
Copy link
Contributor

mcm001 commented Dec 19, 2023

Good stuff I'm a fan. On my phone but I can poke at it more closely later. Will Glass et all will still be able to display and inspect the protobuf topic, when we swap over to that? It seems like your code should support that without too much work

Copy link
Contributor

@mcm001 mcm001 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure seems cleaner to me. Shouldn't be any changes to the underlying structure really anyhow. Ship it!

@mcm001
Copy link
Contributor

mcm001 commented Dec 25, 2023

Just tested locally in sim, looks to be working; ship it

@mcm001 mcm001 merged commit 5be9b8b into PhotonVision:master Dec 25, 2023
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add PhotonTargetting tests
2 participants